Skip to content

fix(resume): write inflight.json atomically - #2708

Merged
Birfy merged 1 commit into
mainfrom
dev/2705-inflight-json-is-truncated-before-it-is
Nov 8, 2026
Merged

fix(resume): write inflight.json atomically#2708
Birfy merged 1 commit into
mainfrom
dev/2705-inflight-json-is-truncated-before-it-is

Conversation

@Birfy

@Birfy Birfy commented Nov 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Fixes #2705

Owner: Dev
Reviewer: Reviewer

inflight.json was written in place: the writer truncated the file and then wrote the new bytes, so a crash inside that window left an empty or half-written record and the next start scheduled no pickup at all — every turn the crash cut off was dropped silently. It is now staged as a temp name beside the file (<pid>.<random>.tmp, at the mode the replaced file already has, 0600 when the name is new) and renamed over inflight.json, so a crash at any point leaves either the previous complete notes or the new complete notes. A file that still cannot be parsed is named once with its path and the reason instead of passing for "nothing was in flight", and the first write of a host sweeps the temp names a dead predecessor left in the home.

core/test/resume-atomic.test.js runs the real install() in child processes in a temporary home and kills it with a real kill -9 while the write is in flight. On the base tree the target is truncated (the window caught was {"target":0}) and the next start schedules nothing; with the change the target still holds the complete previous notes and the pickup is still scheduled ([3000]).

Ran at e2afacd:

  • node scripts/ds-bot-patches.mjs generate (rewrote patches/ds-bot/; only 0001-core-fixes.patch and the new files/test/resume-atomic.test.js differ from main)
  • node scripts/ds-bot-patches.mjs check → ok, and … canonical → ok (generate is a no-op)
  • node core/scripts/build-client.mjs --check → ok
  • node scripts/lint-plugins.mjs → 0 errors
  • cd core && node scripts/lint.mjs → oxlint 0 errors; arch-docs OK (run in a git checkout against this base, because the work copy is a tarball and has no origin/main)
  • node .github/scripts/changelog.mjs check, node .github/scripts/check-mojibake.mjs --check, node .github/scripts/testing-layers.mjs --check → ok
  • cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/resume-atomic.test.js test/follow-through.test.js test/private-files.test.js test/retired-bot-paths.test.js test/task-handoff-route.test.js test/readonly-window-reads.test.js test/connector-access.test.js test/project-board.test.js test/project-board-panels.test.js test/project-board-readonly.test.js test/project-board-view.test.js → 201 tests, 0 fail

Not run: the full npm run verify (left out on purpose per the task brief), the smoke test (no index.js or turn-path change), the plugin suites (nothing under plugins/ changed), and the pack/typecheck steps of the package job.

Docs: docs/subsystems/core-host.md is updated (the private-files paragraph and the restart-pickup bullet), so no Docs-not-needed trailer is needed.

Checkpoint

{
"pr": 2708,
"claim": "Dev",
"issue": 2705,
"summary": "inflight.json was truncated in place before it was written, so a crash inside that window left an empty or half-written record and the next start scheduled no pickup at all. The writer now stages a temp name beside the file and renames it over the old one, so a crash leaves either the previous complete notes or the new complete ones. A file that still cannot be parsed is reported once with its path and reason, and a crashed write's temp name is swept by the next host that writes.",
"files": [
"core/src/host/resume.js",
"core/test/resume-atomic.test.js",
"docs/subsystems/core-host.md",
"changelog.d/2705-inflight-atomic.fixed.md",
"patches/ds-bot/0001-core-fixes.patch",
"patches/ds-bot/files/test/resume-atomic.test.js"
],
"checks": { "ran": "node scripts/ds-bot-patches.mjs check && node scripts/ds-bot-patches.mjs canonical && node core/scripts/build-client.mjs --check", "result": "green", "at": "e2afacd6343a01cc3f88d70cd92662be8aeace46" },
"next": "none — ready for review"
}

@Birfy
fix(resume): write inflight.json atomically
e2afacd 
The notes of a running turn were written in place: the writer truncated the file and then wrote the
new bytes, so a crash in that window left it empty or half written, and the next start scheduled no
pickup at all — every turn the crash cut off was dropped silently (#2705).

Stage a temp name beside the file and rename it over the old one, so a crash at any point leaves
either the previous complete notes or the new complete notes. A file that still cannot be parsed is
named once with its path and the reason instead of passing for "nothing was in flight", and the
first write of a host sweeps the temp names a dead predecessor left behind.

Tests: core/test/resume-atomic.test.js runs the real install() in child processes and kills it with
kill -9 inside the write; the previous writer left the target truncated and scheduled no pickup.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 8, 2026
@Birfy Birfy self-assigned this Nov 8, 2026
@Birfy

Birfy commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at e2afacd
Written-by: Reviewer
Attestation: e2afacd role=Reviewer

Ran at this tip:

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/resume-atomic.test.js test/follow-through.test.js test/private-files.test.js test/retired-bot-paths.test.js test/task-handoff-route.test.js test/readonly-window-reads.test.js test/connector-access.test.js test/project-board.test.js test/project-board-panels.test.js test/project-board-readonly.test.js test/project-board-view.test.js
# tests 201  # pass 201  # fail 0
$ cp <base 651976ff>/core/src/host/resume.js core/src/host/resume.js && cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/resume-atomic.test.js
# tests 3  # pass 0  # fail 3   (kill -9 case: expected 463, actual 0)
$ node scripts/ds-bot-patches.mjs check          # ok 10 patches apply / ok core equals ds-bot + patches
$ node core/scripts/build-client.mjs --check     # ok
$ node .github/scripts/check-mojibake.mjs --check # ok, 0 occurrences of U+FFFD
$ cmp core/test/resume-atomic.test.js patches/ds-bot/files/test/resume-atomic.test.js && cmp core/client.js <base>/core/client.js
# files/ mirror byte-identical; client.js unchanged

Reviewed at the tip tarball (6 files, +540/-27; no work-copy edits other than the red-first swap above, restored and cmp-verified).

  1. Atomicity holds. persist() stages inflight.json.<pid>.<uuid>.tmp via replacePrivateFile (mode = kept mode of the target, 0600 when new) and renames it over the file; the target is never written through. The test runs the real install() in child processes and a real kill -9, and on the base blob it catches the target at 0 bytes (truncated in place) while on the tip the target still holds the complete previous notes and the next start schedules [3000].
  2. Parse failure still does not resume, but is named once. A non-ENOENT read error and a parse/shape failure each warn once with path + reason at install; ENOENT stays silent; no per-write or per-turn warning, so no startup spam. Checked in the child-process reader that collects warns.
  3. Temp hygiene. Name carries pid + random UUID; a rejected temp name throws before staging; a rename failure unlinks the temp; a leftover .tmp is never read as state (the reader only opens inflight.json), and the first write of a writing host sweeps stale inflight.json.*.tmp; a read-only window does not sweep.
  4. No read/schedule semantics moved. The diff is imports, readNotes (error/shape reporting only, same {turns, queued} return), and persist (temp + rename + sweep). RESUME_DELAY_MS = 3000, the turns/queued shapes, taken/firstTurn seeding, the bug(host): a timed-out external room turn leaves its child running, so the next turn receives the stale answer #2634 keep-the-note-while-the-pickup-runs path, QUEUED_MAX/QUEUED_KEPT_MS and the resumeTurn/resendQueued logic are untouched.
  5. Red-first is real: with only core/src/host/resume.js reverted to the base blob and the new test kept, all three new cases fail (the crash case on the truncated target, the warn case on 0 warnings, the sweep case on the stale temp), green again after restoring the tip file.
  6. Generated artifacts agree: patches/ds-bot/files/test/resume-atomic.test.js is byte-identical to core/test/resume-atomic.test.js, ds-bot-patches.mjs check is ok, and core/client.js is unchanged (no core/src/client change).
  7. Regressions: the 201-test affected set is green, including follow-through.test.js (resume/inflight pickup) and the project-board/readonly/connector suites; plugins and A timeline trace that started behind another writer never retries the lock, and flush() reports success while dropping spans #2706's timeline trace are untouched.
  8. Scope: 6 files exactly as claimed, docs/subsystems/core-host.md updated (no Docs-not-needed needed), changelog fragment present; plugins/** unchanged.

Tip checks: tests-ok, plugins (tree|social|timeline|workflow), package, merge-gate, Node 24 · DSH, risk tier, policy-sign, reviewed tip all success; only enqueue-guard is failure and it is exactly "no Verifier report names the tip" — this comment is that report.

Residual notes, not blockers: (a) the sweep unlinks any inflight.json.*.tmp on a host's first write, so two live hosts sharing one home could sweep each other's in-flight temp; that host's write would warn once and be retried on its next change, and the single-host-per-home assumption is already how the file is treated; (b) rename over a file another process holds open can fail on Windows, the trade the old in-place comment described — with reads only at startup in the same host, the failure path is the same warn-and-retry, and crash integrity is the better side of that trade for #2705.

@github-actions github-actions Bot added the pass label Nov 8, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 8, 2026
Merged via the queue into main with commit 42ef470 Nov 8, 2026
17 of 19 checks passed
@Birfy
Birfy deleted the dev/2705-inflight-json-is-truncated-before-it-is branch November 8, 2026 21:06
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Birfy Birfy

Labels

pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

inflight.json is truncated before it is written, so a crash drops every turn it was meant to resume

1 participant

@Birfy